Skip to content

ENH: Ingest MultipleImageIterator into Modules/Core#6263

Merged
hjmjohnson merged 21 commits into
InsightSoftwareConsortium:mainfrom
hjmjohnson:ingest-MultipleImageIterator
May 13, 2026
Merged

ENH: Ingest MultipleImageIterator into Modules/Core#6263
hjmjohnson merged 21 commits into
InsightSoftwareConsortium:mainfrom
hjmjohnson:ingest-MultipleImageIterator

Conversation

@hjmjohnson
Copy link
Copy Markdown
Member

Ingests MultipleImageIterator (upstream KitwareMedical/MultipleImageIterator) into Modules/Core/MultipleImageIterator/ via the v4 two-phase pipeline (#6204). Placed under Modules/Core/ rather than Modules/Filtering/ because iterators are core utilities, matching itkImageRegionIterator placement. Tracking: #6160.

What landed

Modules/Core/MultipleImageIterator/ containing include/itkMultipleImageIterator.h, test/DumpIntensities.cxx, 4 CID-tagged test fixtures, plus CMakeLists.txt, itk-module.cmake, README.md. Mode-A --no-ff --allow-unrelated-histories merge preserves upstream merges and git blame walks to original contributors (Schaerer, Zukić, Birdsong).

Follow-up commits per INGESTION_STRATEGY_v4.md + pre-submission review:

Commit Purpose
DOC: Add README and clean module scaffolding README + replace COMPILE_DEPENDS with DEPENDS ITKCommon + inline DESCRIPTION + add \ingroup ImageIterators topical tag
COMP: Remove .remote.cmake (in-tree) Drop remote-module configure-time fetch
ENH: Enable Module_MultipleImageIterator in configure-ci Add to pyproject.toml CI matrix
Review-driven changes vs upstream

Upstream's itk-module.cmake used COMPILE_DEPENDS (legacy variant), a multi-line set(DOCUMENTATION ...) heredoc, and a single-tag \ingroup MultipleImageIterator. All replaced with the in-tree conventions:

  • DEPENDS ITKCommon (matches every sibling in-tree module)
  • one-line DESCRIPTION "..." inline in itk_module(...)
  • two-tag \ingroup ImageIterators + \ingroup MultipleImageIterator (matches itkImageRegionIterator.h)
Local verification
  • pre-commit run --all-files — ✅ clean
  • pixi run -e cxx configure-ci — ✅ Module_MultipleImageIterator:BOOL=ON
  • cmake --build build -j10 — ✅ 7371/7371 targets (fresh full build)
  • ctest -R MultipleImageIterator — ✅ 3/3 tests pass (KWStyle + DoxygenGroup + computational with ExternalData CID fixtures resolved against data.kitware.com)

Topology check: 4 merges preserved (no linearization per feedback_ingest_merge_topology).

Phase B follow-up (deferred until this PR merges)

After merge:

Utilities/Maintenance/RemoteModuleIngest/archive-remote-module.sh MultipleImageIterator

Phase B publishes the upstream deletion commit on KitwareMedical/MultipleImageIterator, promotes MIGRATION_README.mdREADME.md (per feedback_ingest_archive_readme.md), and flips archived=true. Gated on this PR landing.

dzenanz and others added 16 commits May 24, 2016 11:20
STYLE: Use auto for variable creation

This check is responsible for using the auto type specifier for variable
declarations to improve code readability and maintainability.

The auto type specifier will only be introduced in situations where the
variable type matches the type of the initializer expression. In other words
auto should deduce the same type that was originally spelled in the source

cd /Users/johnsonhj/Dashboard/src/ITK-clangtidy/
run-clang-tidy.py -checks=-*,modernize-use-auto  -header-filter=.* -fix

use auto when declaring iterators
use auto when initializing with a cast to avoid duplicating the type name
use auto when initializing with a template cast to avoid duplicating the type name
use auto when initializing with new to avoid duplicating the type name

SRCDIR=/Users/johnsonhj/Dashboard/src/ITK #My local SRC
BLDDIR=/Users/johnsonhj/Dashboard/src/ITK-clangtidy/ #My local BLD

PERF: Replace explicit return calls of constructor

Replaces explicit calls to the constructor in a return with a braced
initializer list. This way the return type is not needlessly duplicated in the
function definition and the return statement.

SRCDIR=/Users/johnsonhj/Dashboard/src/ITK #My local SRC
BLDDIR=/Users/johnsonhj/Dashboard/src/ITK-clangtidy/ #My local BLD

cd /Users/johnsonhj/Dashboard/src/ITK-clangtidy/
run-clang-tidy.py -checks=-*,modernize-return-braced-init-list  -header-filter=.* -fix

PERF: Allow compiler to choose best way to construct a copy

With move semantics added to the language and the standard library updated with
move constructors added for many types it is now interesting to take an
argument directly by value, instead of by const-reference, and then copy. This
check allows the compiler to take care of choosing the best way to construct
the copy.

The transformation is usually beneficial when the calling code passes an rvalue
and assumes the move construction is a cheap operation. This short example
illustrates how the construction of the value happens:

class Foo {
 public:
-  Foo(const std::string &Copied, const std::string &ReadOnly)
-    : Copied(Copied), ReadOnly(ReadOnly) {}
+  Foo(std::string Moved, const std::string &ReadOnly)
+    : Copied(std::move(Moved)), ReadOnly(ReadOnly) {}
 private:
   private:
   std::string Copied;
   const std::string &ReadOnly;
};

SRCDIR=/Users/johnsonhj/Dashboard/src/ITK #My local SRC
BLDDIR=/Users/johnsonhj/Dashboard/src/ITK-clangtidy/ #My local BLD

cd /Users/johnsonhj/Dashboard/src/ITK-clangtidy/
run-clang-tidy.py -checks=-*,modernize-pass-by-value  -header-filter=.* -fix

STYLE: Use range-based loops from C++11

Used as a more readable equivalent to the traditional for loop operating
over a range of values, such as all elements in a container, in the
forward direction.

====
Range based loopes are more explicit for only computing the
end location once for containers.

for ( ImageIORegion::IndexType::const_iterator i = this->GetIndex().begin();
  i != this->GetIndex().end(); //<- NOTE: Compute end every loop iteration
  ++i )

for (long i : this->GetIndex()) //<- NOTE: Implicitly only compute end once

====
Explicitly reduce the amount of index computations: (The compiler
probably does this too)

for(int i = 0; i < 11; i++)
  {
  pos[0] = testPoints[i][0];
  pos[1] = testPoints[i][1];
                    ^^^^

for(auto & testPoint : testPoints)
  {
  pos[0] = testPoint[0];
  pos[1] = testPoint[1];

====

SRCDIR=/Users/johnsonhj/Dashboard/src/ITK #My local SRC
BLDDIR=/Users/johnsonhj/Dashboard/src/ITK-clangtidy/ #My local BLD

cd /Users/johnsonhj/Dashboard/src/ITK-clangtidy/
run-clang-tidy.py -checks=-*,modernize-loop-convert  -header-filter=.* -fix
ENH: Providing ITKv5 updates for C++11
== http://en.cppreference.com/w/cpp/language/type_alias ==

Type alias is a name that refers to a previously defined type (similar
to typedef).

A type alias declaration introduces a name which can be used as a
synonym for the type denoted by type-id. It does not introduce a new
type and it cannot change the meaning of an existing type name. There is
no difference between a type alias declaration and typedef declaration.
This declaration may appear in block scope, class scope, or namespace
scope.

== https://www.quora.com/Is-using-typedef-in-C++-considered-a-bad-practice ==

While typedef is still available for backward compatibility, the new
Type Alias syntax 'using Alias = ExistingLongName;' is more consistent
with the flow of C++ than the old typedef syntax 'typedef
ExistingLongName Alias;', and it also works for templates (Type alias,
alias template (since C++11)), so leftover 'typedef' aliases will differ
in style from any alias templates.
ENH: Providing ITKv5 updates for C++11
The mission of NumFOCUS is to promote open
practices in research, data, and scientific
computing.

https://numfocus.org
Use correct MultipleImageIterator-Test_LIBRARIES variable instead of
MultipleImageIterator_LIBRARIES for test driver creation.
…update-cid-tags

ENH: Convert ExternalData .md5 tags to .cid (IPFS content IDs)
@github-actions github-actions Bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Enhancement Improvement of existing methods or implementation area:Python wrapping Python bindings for a class area:Core Issues affecting the Core module area:Remotes Issues affecting the Remote module type:Data Changes to testing data labels May 12, 2026
@hjmjohnson hjmjohnson marked this pull request as ready for review May 12, 2026 18:17
@greptile-apps

This comment was marked as resolved.

Comment thread Utilities/Maintenance/RemoteModuleIngest/sanitize-history.py
Comment thread Modules/Core/MultipleImageIterator/test/DumpIntensities.cxx Outdated
hjmjohnson and others added 3 commits May 12, 2026 15:17
Brings MultipleImageIterator from a configure-time remote fetch into the ITK
source tree at Modules/Core/MultipleImageIterator/ using the v4 ingestion
pipeline (whitelist filter-repo + per-commit clang-format + black +
commit-prefix sanitization).

Upstream repo:  https://github.com/KitwareMedical/MultipleImageIterator.git
Upstream tip:   32f67c75cce9be0ceac7876428930be64d99a57b
Ingest date:    2026-05-12
Whitelist:      MultipleImageIterator.list

Per-commit transforms applied across all 16 commits:
  - filter-repo --paths-from-file (whitelist)
  - filter-repo --to-subdirectory-filter Modules/Core/MultipleImageIterator
  - clang-format -style=file (ITK main's .clang-format) for *.cxx/.h/.hxx/...
  - black for *.py
  - heuristic ITK prefix added to commit subjects without one

Merge topology preserved: 8 -> 3 merge(s).

Primary author: Dženan Zukić <dzenan.zukic@kitware.com>

Co-authored-by: Bradley Lowekamp <blowekamp@mail.nih.gov>
Co-authored-by: Hans J. Johnson <hans-johnson@uiowa.edu>
Co-authored-by: Hans Johnson <hans-johnson@uiowa.edu>
Co-authored-by: Hans Johnson <hans.j.johnson@gmail.com>
Co-authored-by: Tom Birdsong <tom.birdsong@kitware.com>
- Add README pointing at archived upstream.
- Replace upstream COMPILE_DEPENDS with DEPENDS ITKCommon (header-only template).
- Inline DESCRIPTION instead of multi-line set(DOCUMENTATION).
- Add \ingroup ImageIterators topical tag.
- operator[] takes unsigned int (matches Size() return type and
  m_Iterators::operator[] signature; prevents silent negative-index UB).
- Remove 'using namespace std;' at global scope and explicitly qualify
  std:: usages in the test driver, per ITK code-style.
@hjmjohnson hjmjohnson force-pushed the ingest-MultipleImageIterator branch from 0c6fac9 to a1a0d48 Compare May 12, 2026 20:17
@github-actions github-actions Bot removed the area:Python wrapping Python bindings for a class label May 12, 2026
@hjmjohnson hjmjohnson merged commit ba1c2b4 into InsightSoftwareConsortium:main May 13, 2026
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Core Issues affecting the Core module area:Remotes Issues affecting the Remote module type:Data Changes to testing data type:Enhancement Improvement of existing methods or implementation type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants